Allow disabling proxy_http_version directive #531
Conversation
The EPEL 6 repository still has version 1.0.15 which does not support this directive proxy_http_version. Yes, this version is old, but a lot of people prefer not to use a bleeding edge version from nginx.org.
ckaenzig
force-pushed
the
optional-proxy-http-version
branch
from
224a550 to
7432862
Compare
| @@ -106,7 +106,9 @@ | |||
| } | |||
| validate_string($multi_accept) | |||
| validate_array($proxy_set_header) | |||
| validate_string($proxy_http_version) | |||
| if ($proxy_http_version != false) { | |||
There was a problem hiding this comment.
Could you make this if ($proxy_http_version != undef)?
At the moment most other parameters that cause directives to be removed from the config files check for an undef value.
|
Looks good, just added that one comment about undef vs false so if you could switch that let's get this merged. Thanks for the contribution! |
|
Your suggestion makes sense, but I couldn't make it work as is. The problem is that nginx::config::proxy_http_version is supposed to be set by hiera, but it looks like hiera is not able to return an undef value. If I set "nginx::config::proxy_http_version: undef" in hiera, it seems I get a String object in puppet (tested with @proxy_http_version.class.name in the ERB template). I'm using puppet-server 3.7.3, hiera 1.3.4. It seems the only way to achieve what I want using undef would be to set the default value of $proxy_http_version to undef. This would remove the proxy_http_version directive in proxy.conf when the class parameter is not set, but the nginx behavior should be unchanged as the defaulf value for this nginx directive is "1.0". Would it be ok to change the default value of $proxy_http_version in the nginx::config class? I'll update this PR shortly with my proposed change. |
The default value in nginx::config was the same as nginx's default, so it's not necessary to be set in any version of nginx unless one needs a specific value. This also allows us to use undef when we don't want the directive to be set instead of false. Also added a test for the default case.
|
That's actually how I'd prefer all the parameters work in this module (default |
Allow disabling proxy_http_version directive
|
Awesome, thanks for merging! |
…rsion Allow disabling proxy_http_version directive
The EPEL 6 repository still has version 1.0.15 which does not support
this directive proxy_http_version. Yes, this version is old, but a lot
of people prefer not to use a bleeding edge version from nginx.org.